Skip to content

Conversation

@jor2
Copy link
Member

@jor2 jor2 commented Feb 20, 2025

Description

Currently the SCC Workload Protection service is provisioned (optionally) as part of the SCC DA.

We want to split it into its own DA. The code should live in the module repo: https://github.com/terraform-ibm-modules/terraform-ibm-scc-workload-protection

After doing this, we should remove support for Workload Protections from the SCC DA, and instead leverage add-ons to make it an optional addon.
We might also want to consider moving the SCC DA code from terraform-ibm-scc-da into the terraform-ibm-scc and remove the scc-da specific repo.

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

@jor2 jor2 self-assigned this Feb 20, 2025
@jor2 jor2 requested review from akocbek and shemau as code owners February 20, 2025 03:17
@jor2
Copy link
Member Author

jor2 commented Feb 20, 2025

/run pipeline

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the ibm_catalog.json and .catalog-onboard-pipeline.yaml files please? Also I think we should name the variation Baseline instead of Standard as it looks like this is the naming convention we are going it. In parallel I am trying to get confirmation n that

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more comments. Also when creating the ibm_catalog.json make sure to follow all the best practises we are rolling out as per https://github.ibm.com/GoldenEye/issues/issues/12542. There is a reference PR which you can copy the approach in (its still under review, but you can see the review comments)

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See latest comments.

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jor2 still several things to address. See comments.
Also we should not be using display_name in the catalog json - please remove them.

In the diagram update Monitoring -> Existing Monitoring instance and on the arrow, put "metrics" on it

tests/pr_test.go Outdated
options := setupOptions(t, "scc-wp-upg", advancedExampleDir)
var region = validRegions[rand.IntN(len(validRegions))]

options := testhelper.TestOptionsDefault(&testhelper.TestOptions{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the new schematics upgrade test instead? All DA tests should use schematics

tests/pr_test.go Outdated
const advancedExampleDir = "examples/advanced"
const basicExampleDir = "examples/basic"
const resourceGroup = "geretain-test-resources"
const standardSolutionDir = "solutions/standard"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be solutions/fully-configurable

assert.NotNil(t, output, "Expected some output")
}

func TestRunAdvancedUpgradeExample(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a normal test - not upgrade test

variable "scc_workload_protection_instance_name" {
description = "The name for the Workload Protection instance that is created by this solution. Must begin with a letter. Applies only if `provision_scc_workload_protection` is true. If a prefix input variable is specified, the prefix is added to the name in the `<prefix>-<name>` format."
type = string
default = "workload_protection"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scc-workload-protection

],
"short_description": "Creates and configures IBM Security and Compliance Center resources",
"long_description": "This architecture supports creating and configuring IBM Security and Compliance Center resources.",
"offering_docs_url": "https://github.com/terraform-ibm-modules/terraform-ibm-scc-workload-protection/blob/main/solutions/instances/README.md",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong directory

ibm_catalog.json Outdated
"solution"
],
"short_description": "Creates and configures IBM Security and Compliance Center resources",
"long_description": "This architecture supports creating and configuring IBM Security and Compliance Center resources.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs to be updated

ibm_catalog.json Outdated
"terraform",
"solution"
],
"short_description": "Creates and configures IBM Security and Compliance Center resources",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs to be updated

ibm_catalog.json Outdated
{
"products": [
{
"name": "deploy-arch-ibm-workload-protection",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe go with deploy-arch-ibm-scc-workload-protection

---
apiVersion: v1
offerings:
- name: deploy-arch-ibm-workload-protection
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe go with deploy-arch-ibm-scc-workload-protection

Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final few comments

variable "region" {
type = string
default = "us-south"
description = "The region to provision Security and Compliance Center resources in."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jor2 still needs to be addressed


This solution supports provisioning and configuring the following infrastructure:

- A resource group, if one is not passed in.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not create a new RG

ibm_catalog.json Outdated
"support_details": "This product is in the community registry, as such support is handled through the originated repo. If you experience issues please open an issue in that repository [https://github.com/terraform-ibm-modules/terraform-ibm-scc-workload-protection/issues](https://github.com/terraform-ibm-modules/terraform-ibm-scc-workload-protection/issues). Please note this product is not supported via the IBM Cloud Support Center.",
"flavors": [
{
"label": "Fully Configurable",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fully configurable

offerings:
- name: deploy-arch-ibm-scc-workload-protection
kind: solution
catalog_id: _
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7df1e4ca-d54c-4fd0-82ce-3d13247308cd

- name: deploy-arch-ibm-scc-workload-protection
kind: solution
catalog_id: _
offering_id: _
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4322cf44-2289-49aa-a719-dd79e39b14dc

tests/pr_test.go Outdated
const advancedExampleDir = "examples/advanced"
const basicExampleDir = "examples/basic"
const resourceGroup = "geretain-test-resources"
const standardSolutionDir = "solutions/fully-configurable"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

standardSolutionDir -> fullyConfigurableDADir

@ocofaigh
Copy link
Contributor

/run pipeline

@ocofaigh ocofaigh merged commit 0c11711 into main Mar 14, 2025
2 checks passed
@ocofaigh ocofaigh deleted the wp-da branch March 14, 2025 12:09
@terraform-ibm-modules-ops
Copy link
Contributor

🎉 This PR is included in version 1.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants